Skip to content

Default privileges support schemas #1300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 21, 2021
Merged

Default privileges support schemas #1300

merged 1 commit into from
Dec 21, 2021

Conversation

fish-face
Copy link
Contributor

WIP because it is dependent on PR 1298 since schema privileges require the schema parameter to be applying to all schemas (schemas can't be nested, so it doesn't make sense to set privileges for new schemas created "within the public schema" or something).

This would also benefit from acceptance tests once I know how to make them work.

@fish-face fish-face requested a review from a team as a code owner September 22, 2021 12:14
@puppet-community-rangefinder
Copy link

postgresql::server::default_privileges is a type

that may have no external impact to Forge modules.

This module is declared in 70 of 578 indexed public Puppetfiles.

These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@fish-face fish-face force-pushed the default_privileges_support_schemas branch from b53234f to 4123399 Compare September 29, 2021 17:16
@fish-face fish-face force-pushed the default_privileges_support_schemas branch from 4123399 to 969d33b Compare October 7, 2021 18:14
@fish-face fish-face force-pushed the default_privileges_support_schemas branch 3 times, most recently from cf5499b to 263c31d Compare October 22, 2021 16:54
@fish-face
Copy link
Contributor Author

So the acceptance tests fail on some environments because of the postgres version being installed on them. I've pushed up something which might fix that - if not I would appreciate some guidance on how to ensure the correct version is used (and if that's not possible, to skip the test in that environment, I suppose?)

@fish-face
Copy link
Contributor Author

Any advice on getting the tests which rely on specific versions working?

@fish-face fish-face force-pushed the default_privileges_support_schemas branch from 263c31d to 97595dd Compare November 15, 2021 17:04
@fish-face fish-face force-pushed the default_privileges_support_schemas branch from 97595dd to 361e448 Compare November 15, 2021 17:24
@fish-face
Copy link
Contributor Author

I think the issue was simply that the check in the module was using versioncmp(..., '10.0') rather than versioncmp(..., '10') - can you confirm that the latter is correct?

ekohl
ekohl reviewed Nov 16, 2021
View reviewed changes
@fish-face fish-face changed the title WIP: Default privileges support schemas Default privileges support schemas Nov 19, 2021
@fish-face
Copy link
Contributor Author

Marked as not WIP.

@fish-face fish-face requested review from ekohl and removed request for a team November 25, 2021 16:59
@fish-face
Copy link
Contributor Author

Hi, aware you are pretty busy - if you are able to have another look at this soon that would be great.

@ekohl
Copy link
Collaborator

ekohl commented Nov 29, 2021

I'm away until next week.

ekohl
ekohl approved these changes Dec 21, 2021
View reviewed changes
Copy link
Collaborator

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not my area of PostgreSQL but the Puppet code looks ok so I'm merging this. Thanks!

@ekohl ekohl merged commit ecf7ad5 into puppetlabs:main Dec 21, 2021
@ekohl ekohl added the feature label Jan 26, 2022
cegeka-jenkins pushed a commit to cegeka/puppet-postgresql that referenced this pull request Feb 3, 2022
@ekohl
Merge pull request puppetlabs#1300 from fish-face/default_privileges_…
ee0e2d9 
…support_schemas

Default privileges support schemas
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ekohl ekohl ekohl approved these changes

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fish-face @ekohl